Just curious on the No JS fallback for this and how it looks?

Looks like the changes have affected certain scenarios on the homepage:

Looks like the changes have affected certain scenarios on the homepage:

Which asset/component is this, is the second image the one that's wrong? @amoore108

Ah figured it out - it's actually effecting a fair few things, fixing those this morning!

Also it doesn't appear to work for me on Safari (there is just a black background). But I assume this is ok for our user base.

Tested on Safari mobile as well and it doesn't seem to work. Feel like this would be an issue as iOS is quite high usage for us.

Also it doesn't appear to work for me on Safari (there is just a black background). But I assume this is ok for our user base.

Tested on Safari mobile as well and it doesn't seem to work. Feel like this would be an issue as iOS is quite high usage for us.

Strange - what version are you on? As it works for me on both safari mobile and desktop, checking https://simorgh1.belfrage-preview.test.api.bbc.com/persian/topics/c6z7mnr559gt?renderer_env=live @Isabella-Mitchell @amoore108

Also it doesn't appear to work for me on Safari (there is just a black background). But I assume this is ok for our user base.

Tested on Safari mobile as well and it doesn't seem to work. Feel like this would be an issue as iOS is quite high usage for us.

Strange - what version are you on? As it works for me on both safari mobile and desktop, checking...>

Preview works for me on Safari desktop and mobile with the preview link. I had only tested local.

Preview works for me on Safari desktop and mobile with the preview link. I had only tested local.

Thanks for the preview link, yea that works on desktop and mobile for me although quite visibly 'jumps' between the image filling the container and going portrait.

Preview works for me on Safari desktop and mobile with the preview link. I had only tested local.

Thanks for the preview link, yea that works on desktop and mobile for me although quite visibly 'jumps' between the image filling the container and going portrait.

Yeahh i think i might check this with UX before merge , my decision process for this was here. The only way to prevent this really is to assume all images are portrait at first, but then this makes landscape images do the visible jumps, and it felt like those are more prominent across the site so we should prioritise those?

Preview works for me on Safari desktop and mobile with the preview link. I had only tested local.

Thanks for the preview link, yea that works on desktop and mobile for me although quite visibly 'jumps' between the image filling the container and going portrait.

Yeahh i think i might check this with UX before merge , my decision process for this was here. The only way to prevent this really is to assume all images are portrait at first, but then this makes landscape images do the visible jumps, and it felt like those are more prominent across the site so we should prioritise those?

Ideally we should avoid the jumping behaviour completely, its happening because of the useEffect re-rendering when it figures out the orientation. Its a shame there isn't a pure CSS way here. Are the background colour gradients necessary at this stage? I assumed we'd just have a quick interim fix for this problem, but this feels more permanent.

Preview works for me on Safari desktop and mobile with the preview link. I had only tested local.

Thanks for the preview link, yea that works on desktop and mobile for me although quite visibly 'jumps' between the image filling the container and going portrait.

Yeahh i think i might check this with UX before merge , my decision process for this was here. The only way to prevent this really is to assume all images are portrait at first, but then this makes landscape images do the visible jumps, and it felt like those are more prominent across the site so we should prioritise those?

Ideally we should avoid the jumping behaviour completely, its happening because of the useEffect re-rendering when it figures out the orientation. Its a shame there isn't a pure CSS way here. Are the background colour gradients necessary at this stage? I assumed we'd just have a quick interim fix for this problem, but this feels more permanent.

Yeahhh i know, ive just tried to play around with setting isPortrait to null iniitally and displaying the placeholder for longer while we figure out if the image is portrait or not, but i couldnt really get it working properly. I also have tried messing around with aspect ratios to see if i can get the pure css working but it doesnt seem possible to account for portrait and landscape this way 😢 I could check if the background colour is necessary, but even without it we'd still have the jumpy problem :/ @amoore108

I can still see some snapshots in Chromatic with diffs that I wouldn't expect.

The other way around this would be to extract the gradient background into its own component and use it directly in the CurationPromo so that it only runs and effects there. This is the way FrostedGlassPromo does it:

It has the Image as a distinct element, with the gradient calcuation in its own component so its isolated. You could create a PromoGlassPanel and use it in CurationPromo so it doesn't affect other components and keeps the Image component more pure. The FrostedGlassPromo wraps the panel part in Lazyload too, so its only loaded when in the viewport, which is useful on mobile.

Its good work here, feels more permanent than temporary though which is why I'm trying to keep the complexity and potential side-effects down. Just don't want a bunch of work to go into a solution that will be taken away? May be worth a huddle with a few folks to discuss?

Hey hey, I know this is temporary solution but just to highlight (if I'm understanding correctly) useImageColour now runs for every Image instance before checking isPromo/isPortraitImage according to the Image component?

I think that will trigger an extra image load and ColorThief run around here:

So there might be a risk that each canonical image (including any lazy loaded/offscreen renders) could start fetching/processing right away, bypassing lazy-load deferral for that extra fetch and that could add significant CPU/network cost across the site. Awesome job, this seems a bit fiddly

I can still see some snapshots in Chromatic with diffs that I wouldn't expect.

The other way around this would be to extract the gradient background into its own component and use it directly in the CurationPromo so that it only runs and effects there. This is the way FrostedGlassPromo does it:

simorgh/src/app/components/FrostedGlassPromo/index.tsx

Lines 138 to 144 in aae34f9

	<FrostedGlassPanel
	image={image.src}
	minimumContrast={minimumContrast}
	paletteSize={paletteSize}
	>
	{promoText}
	</FrostedGlassPanel>

It has the Image as a distinct element, with the gradient calcuation in its own component so its isolated. You could create a PromoGlassPanel and use it in CurationPromo so it doesn't affect other components and keeps the Image component more pure. The FrostedGlassPromo wraps the panel part in Lazyload too, so its only loaded when in the viewport, which is useful on mobile.

Its good work here, feels more permanent than temporary though which is why I'm trying to keep the complexity and potential side-effects down. Just don't want a bunch of work to go into a solution that will be taken away? May be worth a huddle with a few folks to discuss?

Which ones in the chromatic would you not expect? Ive had a quick look through and it looks like it's all just promos that have changed which would be correct - i thought by keeping the isPromo boolean in place it would prevent it making changes to images everywhere - might have missed something though!

Which ones in the chromatic would you not expect? Ive had a quick look through and it looks like it's all just promos that have changed which would be correct - i thought by keeping the isPromo boolean in place it would prevent it making changes to images everywhere - might have missed something though!

Theres a promo on the homepage in the billboard that shouldn't have a background:

Similarly in articles, I wouldn't expect these related content items to have a coloured background:

Editorial do use transparent background images on occasion, so I'm a bit concerned that this logic would always add some kind of background unexpectedly.

Which ones in the chromatic would you not expect? Ive had a quick look through and it looks like it's all just promos that have changed which would be correct - i thought by keeping the isPromo boolean in place it would prevent it making changes to images everywhere - might have missed something though!

Theres a promo on the homepage in the billboard that shouldn't have a background:

Similarly in articles, I wouldn't expect these related content items to have a coloured background:

Editorial do use transparent background images on occasion, so I'm a bit concerned that this logic would always add some kind of background unexpectedly.

think id just missed a conditional off 🤦 , ive added it in now so this shouldnt happen anymore for non portrait, non promo images! checked these two locally and they were back to normal but will double check the chromatic when it's done

Hey hey, I know this is temporary solution but just to highlight (if I'm understanding correctly) useImageColour now runs for every Image instance before checking isPromo/isPortraitImage according to the Image component?

I think that will trigger an extra image load and ColorThief run around here:

simorgh/src/app/hooks/useImageColour/index.js

Line 31 in 330a991

setPalette(colorThief.getPalette(img, paletteSize));

.
So there might be a risk that each canonical image (including any lazy loaded/offscreen renders) could start fetching/processing right away, bypassing lazy-load deferral for that extra fetch and that could add significant CPU/network cost across the site. Awesome job, this seems a bit fiddly

This is a good spot, i think i might have to check whether having the colour pull through is completely necessary with UX, because i think the only way around this would be to have this hook called in another component like the FrostedGlassPanel, and conditionally render that. This feels a bit much for a supposedly temporary solution, when we could just stick with the black or white background without having to call a hook at all?

nvm i think i've prevented this a little by calling it conditionally just for portrait image promos